Warn for non-standard datetime values by ryux1 · Pull Request #378 · github/relative-time-element

ryux1 · 2026-09-07T17:03:13Z

Summary

validate non-empty datetime attributes against the ECMAScript date-time string format
warn for malformed or implementation-dependent values while preserving the existing parsing behavior
cover valid date-only, date-time, offset, expanded-year, and midnight forms

Closes #266

Testing

npm test (636 Chromium tests)
TypeScript and ESLint checks run as part of the test build
npx prettier --check src/relative-time-element.ts test/relative-time.js
git diff --check

Copilot

Copilot review overview

🟡 Changes recommended

Validation remains partly implementation-dependent, and the new test assumes V8-specific parsing behavior.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2

Open (2)

Require complete date before accepting a time suffix · New
Avoid hard-coding engine-specific Date.parse results · New

What changed in this PR

Adds warnings for non-standard datetime values while preserving native parsing behavior.

Changes:

Adds ECMAScript date-time validation and warnings.
Adds coverage for valid and malformed values.

File	Description
`src/relative-time-element.ts`	Adds validation and warning logic.
`test/relative-time.js`	Tests warnings and accepted formats.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+const dateTimeFormatPattern =
+  /^(?:\d{4}|(?!-000000)[+-]\d{6})(?:-(?:0[1-9]|1[0-2])(?:-(?:0[1-9]|[12]\d|3[01]))?)?(?:T(?:(?:[01]\d|2[0-3]):[0-5]\d(?::[0-5]\d(?:\.\d{3})?)?|24:00(?::00(?:\.000)?)?)(?:Z|[+-](?:[01]\d|2[0-3]):[0-5]\d)?)?$/


Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🔵 Needs a closer look

The regex accepts incomplete date-time forms such as 2024T03:04, preventing the intended warning.

Review effort: Balanced
Findings: 1

Open (1)

Require complete date before accepting a time suffix

Resolved since last review (1)

Avoid hard-coding engine-specific Date.parse results

Previously missed (1)

In code that hasn't changed since last review

Require complete dates before accepting a time component

src/relative-time-element.ts:23

The time portion is optional independently of the month/day portion, so non-standard values such as 2024T03:04 and 2024-01T03:04 match this regex. V8's Date.parse also accepts both, which means Chromium emits no warning for exactly the implementation-dependent fallback this validation is intended to detect. Nest the T... group under the day component (and add these values to the malformed cases) so a time is only allowed after a complete date.

francinelucca

Thanks for this contribution @ryux1 🙏🏽, can you take a look at copilot's review comments above? thanks!

Warn for non-standard datetime values

bd7b6f5

ryux1 requested a review from a team as a code owner September 7, 2026 17:03

Merge branch 'main' into fix/strict-datetime-validation

460d35e

francinelucca requested a balanced review from Copilot September 23, 2026 02:54

Copilot started reviewing on behalf of francinelucca September 23, 2026 02:54 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Update datetime assertion to use Date.parse

c1a9afb

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

francinelucca requested a balanced review from Copilot September 23, 2026 03:02

Copilot started reviewing on behalf of francinelucca September 23, 2026 03:02 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

francinelucca reviewed Sep 23, 2026

View reviewed changes

Navigation Menu

Warn for non-standard datetime values - #378

Warn for non-standard datetime values#378
ryux1 wants to merge 3 commits into
github:mainfrom
ryux1:fix/strict-datetime-validation

ryux1 commented Sep 7, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

francinelucca left a comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		const dateTimeFormatPattern =
		/^(?:\d{4}\|(?!-000000)[+-]\d{6})(?:-(?:0[1-9]\|1[0-2])(?:-(?:0[1-9]\|[12]\d\|3[01]))?)?(?:T(?:(?:[01]\d\|2[0-3]):[0-5]\d(?::[0-5]\d(?:\.\d{3})?)?\|24:00(?::00(?:\.000)?)?)(?:Z\|[+-](?:[01]\d\|2[0-3]):[0-5]\d)?)?$/

Conversation

ryux1 commented Sep 7, 2026

Summary

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🔵 Needs a closer look

Uh oh!

francinelucca left a comment

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants